-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Inserter: Try deferring search value instead of debouncing #58058
Conversation
Size Change: +13 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
I am getting mixed reports on CI; locally results are the same, or deferred search performance is slightly better. I would appreciate another set of tests as a control. |
What do you expect from this change? Input debouncing is usually used to prevent doing expensive search (REST requests) on each keystroke. If you type reasonably quickly, there won't be multiple requests like |
It is just an experiment to see if we can avoid a fixed delay. I think the only HTTP request the inserter is making is for the block directory. Using deferred value worked out well in #56139, and I wanted to see if it is a viable replacement for the inserter. |
I'm not sure I agree with this change when it's about limiting network requests. FWIW, we intentionally wanted to reduce the number of requests here, and if you read further in the docs, it says:
IMHO, we don't want this as it's not guaranteed to minimize how often it triggers, and we intentionally want to limit the number of requests per given time limit, as on a faster device it can theoretically still issue too many (unnecessary) network requests. You can think of this experience as searching in settings on an iPhone, where it intentionally adds a small delay before starting to search and it feels very natural, not to mention the performance improvement caused by the fewer rerenders and the reduced server load that's a direct consequence of fewer performed requests to the API. |
Makes sense. I just monitored network requests while searching - it's a lot; using debouncing makes more sense. I appreciate your feedback on my experimental PR 🙇 |
I played with this and it works well 🙂 There are two searches that are triggered by typing in the search field:
I can't type fast enough to prevent the installed blocks happening on every keystroke. But apparently my computer is fast enough for the browser to always have enough idle time to do the search, so it's OK. And it turns out that the REST requests for block directory have their own 400ms debouncing! So, even with this patch applied, the search happens only once, after I'm finished typing my search query. |
Oh, what specifically were you testing and what were the requests? I had a different, much better experience in my testing. |
Using the TT4 theme, try typing |
Oh yes, I forgot about patterns. And was looking only at REST requests (fetch/XHR), so I missed all the requests that the preview iframes are making. |
What?
PR tries to replace
useDebouncedInput
withuseDeferredValue
for global inserter search.Why?
Testing Instructions
image
orpara
Testing Instructions for Keyboard
Same